-
Notifications
You must be signed in to change notification settings - Fork 214
[RFC-0010] Add multi-tenant workload identity support for AWS Bucket #1868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[RFC-0010] Add multi-tenant workload identity support for AWS Bucket #1868
Conversation
d550312
to
3e0213c
Compare
Signed-off-by: cappyzawa <[email protected]>
3e0213c
to
3300705
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good overall, thanks!
@@ -273,6 +273,55 @@ data: | |||
secretkey: <BASE64> | |||
``` | |||
|
|||
##### AWS Controller-Level Workload Identity example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's mention specifically https://fluxcd.io/flux/integrations/aws/#for-amazon-simple-storage-service as well like we did for GCP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add the link eventually. However, the current documentation at that URL (specifically the "At the object level" section) states that S3 integration with the Bucket
API "does not support configuring authentication through OIDC Federation at the object level" and will be introduced in Flux v2.7.
Since this PR is implementing that functionality, I'll include the link in this PR and update the website documentation afterward to reflect the new capabilities. This way users will have the reference ready once the website is updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed via 794228f .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ve made the same update for Azure as well: 3a14ce4
9fe7486
to
ef755b2
Compare
Let's not delete this much documentation on this PR, in the past we considered deleting all Workload Identity related documentation and just linking to the full guide and decided not to do that in that moment, we need more time to think how we will do this. Can you please reset to this commit? 3300705 This one was looking pretty good, I'd like to reboot the review starting from that one |
3a14ce4
to
29b4242
Compare
…Bucket Remove os.Setenv() to prevent env var pollution Remove os.Setenv() call that could cause environment variable pollution in multi-tenant environments. The pkg/auth/aws package already handles region configuration properly by prioritizing opts.STSRegion over AWS_REGION environment variable. Signed-off-by: cappyzawa <[email protected]>
29b4242
to
6dd483d
Compare
Part of: fluxcd/flux2#5022